Skip to content

[kernel] Prevent duplicate defunct enqueue for deleted threads. - #11519

Open
Rbb666 wants to merge 6 commits into
RT-Thread:masterfrom
Rbb666:defunct
Open

Rbb666 wants to merge 6 commits into
RT-Thread:masterfrom
Rbb666:defunct

Conversation

@Rbb666

@Rbb666 Rbb666 commented Jun 26, 2026 •

Copy link
Copy Markdown
Member

拉取/合并请求描述:(PR description)

为什么提交这份 PR (why to submit this PR)

线程自然退出、显式删除或分离后,在对象仍有效但尚未完成回收时再次调用 rt_thread_delete() / rt_thread_detach(),可能将同一线程重复加入 defunct 队列,造成链表破坏或重复清理。本 PR 为每个线程生命周期增加一次性清理保护。

Prevent duplicate defunct enqueue and cleanup while the thread object remains valid.

你的解决方案是什么 (what is your solution)

  • 在 struct rt_thread 中增加独立的 rt_atomic_t defunct,由 _thread_init() 使用 rt_atomic_store() 清零。
  • _thread_mark_defunct() 使用 rt_atomic_exchange() 原子取得清理权,只有新生命周期的初始化才清零。
  • 保持 close → claim → mutex 清理 → defunct 入队的顺序;只有取得清理权的调用执行清理和入队,后续针对仍有效对象的重复 delete/detach 返回 -RT_ERROR。
  • 保护不涵盖已释放句柄,也不解决运行中线程的跨核提前回收问题。依赖线程结构体布局的代码需要重新编译。

Use an independent atomic cleanup claim, initialized with rt_atomic_store() and acquired with rt_atomic_exchange(), so cleanup and enqueue run once per lifetime. Freed handles and remote reclamation of running threads are outside this change.

修改范围 / Changed files

  • include/rtdef.h、src/thread.c:原子清理标志及退出、删除、分离路径保护。
  • src/utest/thread_tc.c:重复操作、静态对象复用及双核并发删除回归。
  • src/utest/thread_suspend_tc.c、components/drivers/ipc/utest/workqueue_tc.c:PR 已有的 SMP 测试顺序修正。
  • .github/workflows/utest_auto_run.yml:PR 已有的 QEMU CPU 数量与日志监控修正。
  • bsp/hk32/hk32f030c8-mini/rtconfig.py:GCC debug 从 -O0 改为 -Og,保留调试信息以适配实际 64 KB Flash。

本轮最小 CI 修复 / Minimal CI follow-up

相对本轮检查开始时的 8406af15df,净改动仅为 HK32 debug 优化选项及 rtdef.h 的 clang-format 空白对齐,共 2 个文件、3 行新增和 2 行删除。没有扩大 Flash 容量或关闭功能。

src/utest/timer_tc.c 已完整恢复,最终 PR 对该文件没有差异。定时器测试的 SMP 同步问题不纳入本次最小修复。

Relative to 8406af15df, this follow-up changes only the HK32 debug optimization and cleanup-field formatting. The timer tests are unchanged in the final PR diff.

回归覆盖 / Regression coverage

  • 动态线程自然退出或显式删除后,在 cleanup 中再次 delete,验证返回 -RT_ERROR 且只清理一次。
  • UP、非 SMART 下,验证重复 detach 和同一静态 TCB 的合法重新初始化;通过后续 cleanup 确认旧生命周期已经回收。
  • SMP 至少两核时,两个 worker 分别绑定 CPU 0/1,并发删除同一未启动动态线程;cleanup 保持对象存活直到在途调用完成,验证一次 RT_EOK、一次 -RT_ERROR 和一次 cleanup。
  • 等待有超时及状态保护;内核调用永久卡死仍需外部超时机制判失败。

请提供验证的 BSP 和 config (provide the config and BSP)

本地验证日期:2026-11-14。工具:Arm GNU GCC 10.3.1、SCons 4.11.1、QEMU ARM 8.0.94、clang-format 22.1.8。配置头均在临时 BSP 副本中通过 .config 生成,未提交构建产物。

验证 / Validation 结果 / Result
HK32F030C8 默认配置普通构建 通过,ROM 42696 B / 64 KB
HK32F030C8 devices.soft_i2c 普通构建 通过,ROM 45696 B / 64 KB
HK32F030C8 devices.soft_i2c 导出工程重新配置、构建 通过,ROM 45696 B / 64 KB
A9 SMP 原定时器测试版本:kernel_basic.cfg、非 SMART、2 核、UTEST 栈 8192 本地严格编译及单次完整运行通过,29/29 testcase
PR 变更范围 clang-format 22.1.8、git diff --check 通过
workflow YAML、7 段 shell 的 bash -n 通过

A9 构建使用 python -m SCons -j8 --strict,运行使用 QEMU -M vexpress-a9 -smp 2。HK32 使用普通构建,仍有既有 GPIO 指针类型警告,不将其报告为严格构建通过;其 CI 工具链为 GCC 13.2.1,与本地不同。

验证范围 / Validation limits

修复前 HK32 CI 的 soft-I²C 镜像为 65632 B,超限 96 B,编译及格式修复有直接失败证据。A9-SMP CI 曾在原有 core.timer 的精确 tick 断言后超时,但本地原版单次运行通过,未复现该次挂死堆栈。本次不宣称修复该偶发运行问题,也不以改写测试用例来消除它。

The original timer tests passed once locally, but CI previously hit an assertion followed by a timeout. That intermittent runtime issue is not claimed as fixed by this minimal follow-up. Local results do not establish coverage of all architectures or the running-thread exit-versus-delete race.

GitHub Actions

提交 49cfd2f1cf14d37e619222114e6cbadb205a101f 的检查已于 2026-11-14 完成:7 个主工作流成功,1 个失败,尚未全部通过。PR Checks 共 109 项成功、4 项按条件跳过、1 项失败,无待执行项。

  • BSP Static Build Check:33/33 编译作业成功。HK32 默认 ROM 42116 B,soft-I²C ROM 45132 B,相关 dist 导出构建也通过;制品汇总按条件跳过。
  • utest_auto_run:46/47 作业成功。A9-SMP kernel_basic 的 29/29 用例通过,包含未改动的 core.timer 以及 core.thread_tc。
  • 格式与许可证、ToolsCI、静态分析、文档、软件包测试、MemBrowse 内存报告均通过。

未通过项 / Outstanding failure

A9-SMP kernel/ipc.cfg 编译通过,但运行在 core.mutex / test_dynamic_pri_reverse 后停止输出,最终触发 20 分钟超时;没有用例断言失败行。

该测试的三个线程非原子递增 volatile int _sync_flag,主线程等待其等于 3,存在 SMP 丢更新后持续等待的风险。CI 没有现场栈或计数值快照,因此这只是与停点相符的风险分析,不能断言已证实此次超时根因。mutex_tc.c 在本 PR 中保持不变,未为此扩大测试修改,也未重跑该失败作业。

Seven main workflows passed and one failed. All 33 BSP build jobs passed; 46 of 47 utest jobs passed. The unchanged A9-SMP IPC mutex test timed out. A non-atomic completion counter is a plausible source-level risk, but the exact hang state was not captured. The minimal compilation/formatting changes remain the only additions in this follow-up.

当前拉取/合并请求的状态 Intent for your PR

必须选择一项 Choose one (Mandatory):

  • 本拉取/合并请求是一个草稿版本 This PR is for a code-review and is intended to get feedback
  • 本拉取/合并请求是一个成熟版本 This PR is mature, and ready to be integrated into the repo

代码质量 Code Quality:

我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:

  • 已经仔细查看过代码改动的对比 Already check the difference between PR and old code
  • 代码风格正确,包括缩进空格,命名及其他风格 Style guide is adhered to, including spacing, naming and other styles
  • 没有垃圾代码,代码尽量精简,不包含#if 0代码,不包含已经被注释了的代码 All redundant code is removed and cleaned up
  • 所有变更均有原因及合理的,并且不会影响到其他软件组件代码或BSP All modifications are justified and not affect other components or BSP
  • 对难懂代码均提供对应的注释 I've commented appropriately where code is tricky
  • 代码是高质量的 Code in this PR is of high quality
  • 已经使用formatting 等源码格式化工具确保格式符合RT-Thread代码规范 This PR complies with RT-Thread code specification
  • 如果是新增bsp, 已经添加ci检查到.github/ALL_BSP_COMPILE.json 详细请参考链接BSP自查

@github-actions

github-actions Bot commented Jun 26, 2026 •

Copy link
Copy Markdown

👋 感谢您对 RT-Thread 的贡献!Thank you for your contribution to RT-Thread!

为确保代码符合 RT-Thread 的编码规范,请在你的仓库中执行以下步骤运行代码格式化工作流(如果格式化CI运行失败)。
To ensure your code complies with RT-Thread's coding style, please run the code formatting workflow by following the steps below (If the formatting of CI fails to run).


🛠 操作步骤 | Steps

  1. 前往 Actions 页面 | Go to the Actions page
    点击进入工作流 → | Click to open workflow →

  2. 点击 Run workflow | Click Run workflow

  • Use workflow from 保持默认分支(通常为 master)
    Keep the default branch (usually master) in Use workflow from
  • 在 branch 输入框填写 PR 分支 defunct
    Enter PR branch defunct in the branch field
  • 设置需排除的文件/目录(目录请以"/"结尾)
    Set files/directories to exclude (directories should end with "/")
  1. 等待工作流完成 | Wait for the workflow to complete
    格式化后的代码将作为独立提交推送至你的分支。
    The formatting changes will be pushed to your branch as a separate commit.

完成后,提交将自动更新至 defunct 分支,关联的 Pull Request 也会同步更新。
Once completed, commits will be pushed to the defunct branch automatically, and the related Pull Request will be updated.

如有问题欢迎联系我们,再次感谢您的贡献!💐
If you have any questions, feel free to reach out. Thanks again for your contribution!

@github-actions

github-actions Bot commented Jun 26, 2026 •

Copy link
Copy Markdown

📌 Code Review Assignment

🏷️ Tag: components

Reviewers: @Maihuanyi

Changed Files (Click to expand)
  • components/drivers/ipc/utest/workqueue_tc.c

🏷️ Tag: kernel

Reviewers: @GorrayLi @ReviewSun @hamburger-os @lianux-mm @wdfk-prog @xu18838022837

Changed Files (Click to expand)
  • src/thread.c
  • src/utest/thread_suspend_tc.c
  • src/utest/thread_tc.c

🏷️ Tag: workflow

Reviewers: @Rbb666 @kurisaW @supperthomas

Changed Files (Click to expand)
  • .github/workflows/utest_auto_run.yml

📊 Current Review Status (Last Updated: 2026-11-14 21:32 CST)


📝 Review Instructions

  1. 维护者可以通过单击此处来刷新审查状态: 🔄 刷新状态
    Maintainers can refresh the review status by clicking here: 🔄 Refresh Status

  2. 确认审核通过后评论 LGTM/lgtm
    Comment LGTM/lgtm after confirming approval

  3. PR合并前需至少一位维护者确认
    PR must be confirmed by at least one maintainer before merging

ℹ️ 刷新CI状态操作需要具备仓库写入权限。
ℹ️ Refresh CI status operation requires repository Write permission.

@github-actions github-actions Bot added the Kernel PR has src relate code label Jun 26, 2026
@github-actions

github-actions Bot commented Jun 26, 2026 •

Copy link
Copy Markdown

MemBrowse Memory Report · Explore what changed →

at32f415-start

  • RAM: .bss +16 B (+0.3%, 4,588 B / 32,768 B, total: 14% used)
  • ROM: .text +176 B (+0.3%, 65,700 B / 262,144 B, total: 25% used)

esp32-c3

  • dram0_0_seg: .dram0.bss +16 B (+0.0%, 216,848 B / 325,120 B, total: 67% used)
  • drom0_0_seg: .eh_frame +48 B, .eh_frame_hdr +8 B (+0.0%, 725,964 B / 8,388,576 B, total: 9% used)

gd32105r-start

  • CODE: .text +176 B (+0.3%, 69,612 B / 262,144 B, total: 27% used)
  • DATA: .bss +8 B (+0.2%, 3,756 B / 98,304 B, total: 4% used)

hc32f334

  • FLASH: .text +188 B (+0.2%, 99,880 B / 131,072 B, total: 76% used)
  • RAM: .bss +8 B (+0.2%, 5,336 B / 32,768 B, total: 16% used)

hpmicro-hpm5100evk

  • DLM: .bss +8 B (+0.0%, 57,904 B / 131,072 B, total: 44% used)
  • ILM: .fast +96 B (+0.2%, 42,768 B / 262,144 B, total: 16% used)
  • XPI0: .fast +96 B (+0.1%, 103,076 B / 1,048,576 B, total: 10% used)

hpmicro-hpm5301evklite

  • DLM: .bss +16 B (+0.0%, 58,576 B / 131,072 B, total: 45% used)
  • ILM: .fast +96 B (+0.4%, 23,824 B / 131,072 B, total: 18% used)
  • XPI0: .fast +96 B (+0.1%, 76,884 B / 1,048,576 B, total: 7% used)

infineon-psoc6

  • flash: .text +176 B (+0.2%, 116,468 B / 262,144 B, total: 44% used)

k230

  • SRAM: .bss +32 B, .eh_frame +56 B (+0.0%, 1,391,544 B / 60,682,240 B, total: 2% used)

loongson-ls1cdev

  • Code: .text +312 B (+0.1%, 364,260 B)
  • Data: .bss +16 B (+0.0%, 59,824 B)

nordic-nrf51822

  • FLASH: .text +168 B (+0.3%, 52,300 B / 262,144 B, total: 20% used)
  • RAM: .bss +4 B (+0.1%, 2,720 B / 16,384 B, total: 17% used)

nuvoton-m487

  • CODE: .text +96 B (+0.0%, 357,008 B / 524,288 B, total: 68% used)
  • DATA: .bss +24 B (+0.0%, 101,952 B / 163,840 B, total: 62% used)

qemu-virt64-aarch64

  • Code: .eh_frame +56 B, .text +240 B (+0.0%, 1,011,492 B)
  • Data: .bss +56 B, .data -8 B (+0.0%, 1,068,168 B)

raspberry-pico-rp2040

  • FLASH: .text +152 B (+0.1%, 114,424 B / 2,097,152 B, total: 5% used)
  • RAM: .bss +4 B (+0.1%, 5,712 B / 262,144 B, total: 2% used)

renesas-ra2l1

  • FLASH: .text +108 B (+0.1%, 98,600 B / 262,144 B, total: 38% used)
  • RAM: .bss +8 B (+0.1%, 6,648 B / 32,768 B, total: 20% used)

simulator

  • Code: .eh_frame +96 B, .eh_frame_hdr +24 B, .rela.dyn +936 B, .rodata +256 B, .text +679 B (+0.1%, 2,678,645 B)
  • Data: .bss +32 B, .data +1,088 B (+0.1%, 1,611,972 B)

stm32f407-rt-spark

  • CODE: .text +176 B (+0.2%, 86,344 B / 1,048,576 B, total: 8% used)
  • RAM1: .bss +8 B (+0.1%, 5,480 B / 131,072 B, total: 4% used)

stm32l475-atk-pandora-llvm

  • RAM1: .bss +8 B (+0.2%, 4,252 B / 98,304 B, total: 4% used)
  • ROM: .text +156 B (+0.2%, 82,960 B / 524,288 B, total: 16% used)

wch-ch32v208w-r0

  • FLASH: .text +104 B (+0.1%, 160,080 B / 491,520 B, total: 33% used)
  • RAM: .bss +8 B (+0.0%, 47,952 B / 65,536 B, total: 73% used)

x86

  • Code: .text +176 B (+0.1%, 199,453 B)
  • Data: .bss +64 B (+0.4%, 14,612 B)

xuantie-e901plus

  • DSRAM: .bss +12 B (+0.1%, 14,016 B / 524,288 B, total: 3% used)

No memory changes detected for:


Catch memory regressions in CI with MemBrowse. Free for open source.

@Rbb666
Rbb666 requested a review from BernardXiong June 27, 2026 03:42

@BernardXiong BernardXiong left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review / 审查

结论 / Conclusion: no blocking issue found in the duplicate-defunct protection.

The new flag is initialized with the scheduler context, and _thread_mark_defunct() makes the check-and-set atomic under the thread spinlock before mutex detachment and defunct enqueue. This addresses the duplicate enqueue/double cleanup scenario described by the PR, and the change is small and localized.

建议 / Suggestions

  1. Please add an SMP-focused regression case, or document why the existing lock ordering is sufficient. The paths now combine the scheduler critical section, the thread spinlock, and mutex detachment; a test should exercise concurrent rt_thread_delete()/rt_thread_detach() against natural thread exit.

  2. The new bit lives in rt_sched_thread_ctx, whose other fields are generally accessed under scheduler-lock rules, while _thread_mark_defunct() uses the thread spinlock. Please add a short comment documenting that sched_flag_defunct is intentionally protected by thread->spinlock, so future scheduler changes do not introduce a lock-domain mismatch.

  3. Keep the negative return behavior covered for both dynamic and static threads after natural exit, including repeated calls. The current tests cover the main cases; the SMP/race case is the remaining useful gap.

The PR description and test intent are clear. / PR 描述和测试意图清晰。

@Rbb666

Rbb666 commented Oct 17, 2026

Copy link
Copy Markdown
Member Author

@BernardXiong 熊大,已经按照修改建议修复了

@Rbb666
Rbb666 force-pushed the defunct branch 2 times, most recently from 745bcd9 to ec6c0ec Compare October 17, 2026 08:04
@Rbb666 Rbb666 closed this Nov 2, 2026
@Rbb666 Rbb666 reopened this Nov 2, 2026
@github-actions github-actions Bot added the action github action yml imporve label Nov 2, 2026
Comment thread include/rtdef.h Outdated
#endif /* RT_USING_MEM_PROTECTION */

struct rt_spinlock spinlock;
rt_uint8_t defunct; /**< cleanup claimed, protected by spinlock */

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里是否用一个原子操作类型,是否也可以?这样可以做无锁化处理

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

好的,我更新下

@Rbb666

Rbb666 commented Nov 7, 2026

Copy link
Copy Markdown
Member Author

@BernardXiong 已更新,采用无锁方式实现,请继续审核

@Rbb666

Rbb666 commented Nov 13, 2026

Copy link
Copy Markdown
Member Author

@BernardXiong

@Rbb666
Rbb666 requested a review from BernardXiong November 13, 2026 01:40
Replace the thread spinlock check-and-set with rt_atomic_exchange and
initialize the claim with rt_atomic_store. Keep the existing cleanup
ordering and repeated delete/detach error behavior.

Sync with master and preserve loop-scaled monitor timeouts, assertion
failure detection and process-substitution log reading.

Validation: workflow YAML parsing, bash -n for all seven shell steps,
and 11 log-monitor scenarios.

@BernardXiong BernardXiong left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

结论:需要修改(仅格式问题);逻辑方向正确。 / Verdict: needs a format fix; the logic looks correct.

  1. [阻塞] CI 格式检查失败:Scan code format and license 失败。请对新增/修改的 src/utest/thread_tc.c、src/utest/thread_suspend_tc.c、components/drivers/ipc/utest/workqueue_tc.c 按仓库 tools 下的 clang-format 配置格式化后重推。others_ft32_mm32_acm32 的失败看起来停在工具链/依赖阶段,但格式化必须先处理。

  2. [确认无误] defunct 认领:_thread_mark_defunct() 用 rt_atomic_exchange() 只允许一个清理者执行 _thread_detach_from_mutex() 和 rt_thread_defunct_enqueue();_thread_init() 在新生命周期开始时清零,静态/动态线程都会经过这里,逻辑闭合。
    _thread_detach() 中 rt_thread_close() 目前恒返回 RT_EOK,因此把入队放进 if (error == RT_EOK) 不改变既有行为。

  3. [建议] 用例覆盖:utest 已覆盖双路径竞争,请确认在 others_ft32_mm32_acm32 失败排除后,带 RT_USING_SMP 的 QEMU 任务能实际跑过该用例,并在 PR 描述里贴一次运行结果。

English: the atomic cleanup claim is sound and _thread_init() resets it per lifetime. The only blocking item is the failing format check; please clang-format the new utest files. The others_ft32_mm32_acm32 failure looks environmental, but confirm the new SMP test actually runs and paste the result.

@BernardXiong

Copy link
Copy Markdown
Member

补充一个当前 CI 的阻塞项:others_ft32_mm32_acm32 在 bsp/hk32/hk32f030c8-mini 上链接失败。

ld: rtthread.elf section `.text' will not fit in region `ROM'
ld: region `ROM' overflowed by 92 bytes
ROM: 65628 B / 64 KB (100.14%)

同一个 job 在 master f601ffbdc4 上是成功的,ROM 占用 65512 B(99.96%,只剩 24 B)。本 PR 新增的 defunct 字段与 _thread_mark_defunct()(src/thread.c:119、include/rtdef.h:960)带来约 116 B 增量,刚好越过该 BSP 的 64 KB 上限。

失败 job:https://github.com/RT-Thread/rt-thread/actions/runs/43348269374/job/131646192628

建议在合并前处理这个 BSP 的体积回归:可以压缩这条 cleanup 路径的代码体积,或与该 BSP 维护者确认是否调整链接脚本/默认配置的 ROM 预算。格式检查失败仍在,按上一条 review 处理即可。

@Rbb666 Rbb666 closed this Nov 14, 2026
@Rbb666 Rbb666 reopened this Nov 14, 2026
@github-actions github-actions Bot added the BSP label Nov 14, 2026

@BernardXiong BernardXiong left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

结论:格式问题已修复;新出现的 A9-smp 超时看起来与本轮增量无关,但 CI 未通过,故本轮不做完整审查。 / Verdict: format fix confirmed; the new A9-smp timeout appears unrelated to this delta, but CI is red so no full review this round.

head 49cfd2f1 相对上一轮审查的 8406af15 仅剩 hk32 调试优化 -O0 → -Og 与 rtdef.h 空白对齐,clang-format 问题已解决。

新失败 / New failure: A9-smp :kernel/ipc.cfg 在 utest 运行 20 分钟后超时,日志停在 core.mutex 的 test_dynamic_pri_reverse;本 PR 的 workqueue_tc 在同一 run 中已通过,且上一轮 head 8406af15 的同一 job 也通过。考虑到增量只有 -Og 与空白,暂判断为间歇性超时而非本 PR 逻辑引入:

建议重跑该配置;若复现,请作者结合 src/thread.c 的 defunct 清理路径定位。本轮不把该 SHA 记为已完整审查。

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants